Conversation
ElliottKasoar
left a comment
ElliottKasoar
left a comment
There was a problem hiding this comment.
I think this has an unfortunate addition to refactoring the callbacks, since the pattern is essentially the same as other user changes (input -> store -> input -> store -> ...) but I think is less fragile for similar reasons to the temporary change we made, so is probably ok for now.
It also has a side effect of removing the possibility of benchmark tables including models not included in the summary, which is currently possible. This was a bit of a legacy from when summary tables couldn't deal with missing values, so this is probably fine, but worth noting, as it does enforce a full set of models in the yml.
| Button( | ||
| "Select all", | ||
| id="model-filter-select-all", | ||
| n_clicks=0, | ||
| style={ | ||
| "fontSize": "11px", | ||
| "padding": "4px 8px", | ||
| "backgroundColor": "#6c757d", | ||
| "color": "white", | ||
| "border": "none", | ||
| "borderRadius": "3px", | ||
| "cursor": "pointer", | ||
| }, | ||
| ), | ||
| Button( | ||
| "Clear", | ||
| id="model-filter-clear-all", | ||
| n_clicks=0, | ||
| style={ | ||
| "fontSize": "11px", | ||
| "padding": "4px 8px", | ||
| "backgroundColor": "#6c757d", | ||
| "color": "white", | ||
| "border": "none", | ||
| "borderRadius": "3px", | ||
| "cursor": "pointer", | ||
| }, | ||
| ), |
There was a problem hiding this comment.
Do we really need these when they're built into the dropdown? It adds extra complexity to the callbacks
2 participants
Pre-review checklist for PR author
PR author must check the checkboxes below when creating the PR.
Summary
Model selection using a dropdown. made a new PR as the old one was as super messy rebase and i gave up
Linked issue
Resolves #190
Testing